-
-
Notifications
You must be signed in to change notification settings - Fork 404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
xgboost: expose watchlist and callbacks #1859
Conversation
…et default lambda=1; add tweedie_variance_power param
R/RLearner_classif_xgboost.R:43:7: style: TODO comments should be removed. # TODO: uncomment the following after the next CRAN update, and set max_depth's lower = 0L
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ R/RLearner_regr_xgboost.R:43:7: style: TODO comments should be removed. # TODO: uncomment the following after the next CRAN update, and set max_depth's lower = 0L
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
@mb706 Can we have the lintr allow TODO comments please? |
that would be direct violation of the style guide <https://github.com/rdatsci/PackagesInfo/wiki/R-Style-Guide> point 17 ;-)
Anhway, to disable this, comment out the "todo.comment" linter here: <https://github.com/mlr-org/mlr/blob/master/tests/testthat/helper_lint.R#L269>
…On June 20, 2017 4:13:43 PM GMT+02:00, Lars Kotthoff ***@***.***> wrote:
@mb706 Can we have the lintr allow TODO comments please?
|
That style point doesn't apply here -- it's a todo for when a new package version is released. The style guide doesn't say anything about not allowing TODOs in general. |
TODO codetags have fairly different semantics (informal tasks pending completion) to FIXME's (something bad or suspicious that needs cleanup, usually before a release). To me, the style guide's p17 simply emphasizes this distinction, even if in a potentially confusing way. |
@khotilov Could you please disable the TODO lintr as suggested so we can see if the build succeeds then? |
@larskotthoff The build has passed. |
Thanks! Merging. |
* xgboost: expose watchlist and callbacks; remove silent from params; set default lambda=1; add tweedie_variance_power param * disable TODO linter
watchlist
andcallbacks
through parameters classif.xgboost - error if early.stop.round is set? #1264silent
from the parameters - it's overridden byverbose
in xgboost R interface and has no effectlambda=1
as it is such for tree boosterstweedie_variance_power
parameter